share: Support switching accounts #1883

rajveermalviya · 2025-09-30T17:49:19Z

Stacked on top of #1860.

Separated from #1816 to preserve some drafts commits there from @chrisbobbe.

Screenshots coming..

Tests are TODO and will happen as part of #1787 in a follow-up PR.

Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=12853-76543&p=f&t=oBRXWxFjbkz1yeI7-0
Fixes: #1779

NFC, because GetServerSettingsResult.realmIcon was never being read.

Fixes-partly: zulip#1036

This will populate these fields for the currently opened account in the database when the PerAccountStore loads.

Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=12853-76543&p=f&t=oBRXWxFjbkz1yeI7-0

Co-Authored-By: Chris Bobbe <[email protected]> Fixes: zulip#1779

rajveermalviya · 2025-10-03T16:27:01Z

Screenshots:

Channels	DMs	Choose account

chrisbobbe

Thanks! Comments below.

chrisbobbe · 2025-10-07T21:55:19Z

lib/widgets/share.dart

          SubscriptionListPageBody(
            showTopicListButtonInActionSheet: false,
            hideChannelsIfUserCantSendMessage: true,
-            allowGoToAllChannels: false,


This removal looks unintentional.

chrisbobbe · 2025-10-07T22:00:42Z

lib/widgets/share.dart

+    // We should already have the `store.realmIcon` after the
+    // PerAccountStore has completed loading, hence the `!` here.
+    final realmIconUrl = store.realmUrl.resolveUri(store.realmIcon!);


If a PerAccountStore is guaranteed to have non-null .realmIcon, then why is it typed as optional? If we can make it required, we should do so, right? (Ditto .realmName.) Then if there's some subtlety in explaining that, we can do it just once where the field is declared, instead of all the places that try to consume it.

chrisbobbe · 2025-10-07T22:13:39Z

lib/widgets/share.dart

+          SizedBox.square(
+            dimension: 42,
+            child: Padding(
+              padding: const EdgeInsets.all(7),
+              child: RealmContentNetworkImage(realmIconUrl))),


Following the link to the "company-logo" component in Figma—

https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=421-14693&m=dev

—it looks like this is supposed to have a 4px border radius:

Would the AvatarShape widget be appropriate here? Its dartdoc says "A rounded square shape, to wrap an [AvatarImage] or similar.".

I also see an additional 4px horizontal padding in the Figma that's not implemented in this revision:

https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=12853-76635&m=dev

chrisbobbe · 2025-10-07T22:21:39Z

lib/widgets/share.dart

+            labelStyle: labelStyle,
+            labelColor: designVariables.iconSelected,
+            unselectedLabelColor: designVariables.icon,
+            indicatorWeight: 0,


Hmm, two things stand out to me in the dartdoc of indicatorWeight:

/// The value of this parameter must be greater than zero.

/// If [indicator] is specified […] this property is ignored.

What happens if we don't pass indicatorWeight?

chrisbobbe · 2025-10-07T22:23:42Z

lib/widgets/share.dart

+    final labelStyle = TextStyle(
+      fontSize: 18,
+      height: 24 / 18,
+      letterSpacing: 0,


Isn't letterSpacing: 0 the default, from zulipTypography?

chrisbobbe · 2025-10-07T23:36:51Z

lib/widgets/share.dart

+                child: RealmContentNetworkImage(realmIconUrl))),
+          ),


Suggested change

child: RealmContentNetworkImage(realmIconUrl))),

),

child: RealmContentNetworkImage(realmIconUrl)))),

chrisbobbe · 2025-10-08T20:59:54Z

lib/widgets/share.dart

+    ShareDialog.show(
+      pageContext: context,
+      initialAccountId: accountId,
+      sharedFiles: sharedFiles,
+      sharedText: intentSendEvent.extraText);


share: Re-design from page to a dialog

This commit breaks the "New DM" button in the "Direct messages" tab, because a PageRoot ancestor is no longer found when the button is tapped:

======== Exception caught by gesture =============================================================== The following assertion was thrown while handling a gesture: No PageRoot ancestor 'package:zulip/widgets/page.dart': Failed assertion: line 26 pos 12: 'element != null' When the exception was thrown, this was the stack: #2 PageRoot.contextOf (package:zulip/widgets/page.dart:26:12) #3 showNewDmSheet (package:zulip/widgets/new_dm_sheet.dart:17:32) #4 _NewDmButtonState.build.<anonymous closure> (package:zulip/widgets/recent_dm_conversations.dart:270:20) [etc.]

chrisbobbe · 2025-10-08T22:06:07Z

lib/widgets/share.dart

+      unawaited(() async {
+        final GetServerSettingsResult serverSettings;
+        final connection = globalStore.apiConnection(
+          realmUrl: account.realmUrl,
+          zulipFeatureLevel: null);
+        try {
+          serverSettings = await getServerSettings(connection);
+        } catch (_) {
+          return;
+        } finally {
+          connection.close();
+        }
+
+        if (globalStore.getAccount(accountId) != null) {
+          await globalStore.updateRealmData(
+            accountId,
+            realmName: serverSettings.realmName,
+            realmIcon: serverSettings.realmIcon);
+        }
+      }());


The goal is to make sure the realm name and icon are present and up-to-date, right?

I think my ideal way to do this would be more centralized and a bit more rigorous:

The choose-account page/dialog doesn't yet show the name and icon, but it will, and it'll be equally useful there to refresh them as here.

We've written careful "refuse-to-connect-to-ancient-servers" logic that should eliminate ancient-server behavior as a possible source of bugs, which should help simplify debugging. This revision puts a small hole in that logic, which we could fix by checking ZulipVersionData.isUnsupported before interpreting realmName and realmIcon. Like we do around the other getServerSettings call.

I don't think any ancient servers format realmName and realmIcon in weird, interfering ways…but the point is being confident that it wouldn't matter even if they did.

The realm name and icon are also provided via the event system, and we should defer to that: if a PerAccountStore finishes loading before we've received and recorded the data from get-server-settings, we should abort instead of clobbering the store with this data that arrived out-of-band.

more centralized

I'm not sure yet where's the most helpful place to put this. Maybe in a stateful widget that gets passed to PerAccountStoreWidget.placeholder? I'd be happy to think about that when I revisit my draft commits in #1816, and in the meantime perhaps you can just address my second and third bullet points above, which are correctness issues.

I think a good home for this would be as a method on GlobalStore, perhaps named like ensureFreshAccountData. (Or ensureFreshRealmMetadata? The point is it's the data (a) that we store in the Account record but (b) about the realm as a whole.)

Then that method will have access to _perAccountStoresLoading as well as _perAccountStores. It can look at those in order to ensure that it doesn't do anything if there's already a PerAccountStore or one being loaded.

chrisbobbe · 2025-10-08T22:23:57Z

lib/widgets/share.dart

+          return const SizedBox.shrink();
+        }
+
+        return ListTile(


Perhaps add a TODO(#1038) for aligning this dialog with other choose-account dialogs (link: #1038)

chrisbobbe · 2025-10-08T22:28:22Z

lib/widgets/share.dart

+            onTap: hasMultipleAccounts
+              ? () {
+                ChooseAccountForShareDialog.show(
+                  pageContext: context,
+                  selectedAccountId: store.accountId,
+                  sharedFiles: sharedFiles,
+                  sharedText: sharedText);
+              }


I wonder if there's some way we can make it clearer that the org icon can be tapped to switch accounts. Like the chevron-down icon on the emoji selector on the set-status page:

chrisbobbe · 2025-10-08T22:32:53Z

@alya curious for your thoughts on #1883 (comment) too.

rajveermalviya and others added 9 commits September 30, 2025 19:53

api: Add realmName and realmIconUrl to InitialSnapshot

5dbcb34

api: Change GetServerSettingsResult.realmIcon type to Uri, from String

52b3bbb

example data [nfc]: Use _realmIcon as default value in eg.serverSettings

ab52325

NFC, because GetServerSettingsResult.realmIcon was never being read.

db: Allow storing realmName and realmIcon to the Accounts table

96e87ff

Fixes-partly: zulip#1036

login: Store realmName and realmIcon while creating account

415b1e9

store: Update account with realmName and realmIcon from InitialSnapshot

f3e6429

This will populate these fields for the currently opened account in the database when the PerAccountStore loads.

store [nfc]: Add getters for realmName and realmIcon

765d19e

share: Re-design from page to a dialog

6b9694c

Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=12853-76543&p=f&t=oBRXWxFjbkz1yeI7-0

share: Support switching accounts

01bc09d

Co-Authored-By: Chris Bobbe <[email protected]> Fixes: zulip#1779

rajveermalviya mentioned this pull request Sep 30, 2025

share: Support choosing an account, if there are multiple #1816

Closed

rajveermalviya added the maintainer review PR ready for review by Zulip maintainers label Sep 30, 2025

rajveermalviya assigned chrisbobbe Sep 30, 2025

rajveermalviya requested a review from chrisbobbe September 30, 2025 17:51

chrisbobbe reviewed Oct 8, 2025

View reviewed changes

	child: RealmContentNetworkImage(realmIconUrl))),
	),
	child: RealmContentNetworkImage(realmIconUrl)))),

share: Support switching accounts #1883

Are you sure you want to change the base?

share: Support switching accounts #1883

Uh oh!

Conversation

rajveermalviya commented Sep 30, 2025

Uh oh!

rajveermalviya commented Oct 3, 2025

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisbobbe commented Oct 8, 2025

Uh oh!

Uh oh!

chrisbobbe Oct 8, 2025 •

edited

Loading